Skip to content

Fix GDPopt LBB time-limit results#3942

Open
bernalde wants to merge 2 commits into
Pyomo:mainfrom
SECQUOIA:fix/issue-3941-gdpopt-lbb-time-limit
Open

Fix GDPopt LBB time-limit results#3942
bernalde wants to merge 2 commits into
Pyomo:mainfrom
SECQUOIA:fix/issue-3941-gdpopt-lbb-time-limit

Conversation

@bernalde
Copy link
Copy Markdown
Contributor

Summary

  • Replace GDPopt LBB's stale _get_final_results_object() call on the time-limit path with _get_final_pyomo_results_object().
  • Add a solver-independent LBB regression test that forces the time-limit branch and verifies maxTimeLimit results.
  • Add MindtPy GOA time-limit coverage to verify its inherited GOA path sets maxTimeLimit on SolverResults.

Tests run

  • pytest -q pyomo/contrib/gdpopt/tests/test_LBB.py::TestGDPopt_LBB_TimeLimit::test_time_limit_returns_pyomo_results_object pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py::TestMindtPyGOATimeLimit::test_goa_time_limit_sets_solver_results_condition - passed, 2 passed in 0.52s
  • pytest -q pyomo/contrib/gdpopt/tests/test_LBB.py pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py - passed, 32 passed, 9 skipped, 2 deselected in 1.86s
  • python -m black --check --diff pyomo/contrib/gdpopt/branch_and_bound.py pyomo/contrib/gdpopt/tests/test_LBB.py pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py - passed, 3 files would be left unchanged
  • git diff --check -- pyomo/contrib/gdpopt/branch_and_bound.py pyomo/contrib/gdpopt/tests/test_LBB.py pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py - passed

Notes about tests not run

  • Full repository pytest and full-repository Black were not run locally because the targeted change is narrow and the full CI matrix covers multiple Python versions, platforms, optional solver installs, documentation, and coverage jobs.

Closes #3941

Copy link
Copy Markdown
Contributor Author

@bernalde bernalde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

  1. Blocking issues: none.
  2. Nonblocking issues: none.
  3. Questions: none.
  4. Tests run:
    • pytest -q pyomo/contrib/gdpopt/tests/test_LBB.py::TestGDPopt_LBB_TimeLimit::test_time_limit_returns_pyomo_results_object pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py::TestMindtPyGOATimeLimit::test_goa_time_limit_sets_solver_results_condition - passed, 2 passed in 0.50s.
    • pytest -q pyomo/contrib/gdpopt/tests/test_LBB.py pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py - passed, 32 passed, 9 skipped, 2 deselected in 1.69s.
    • python -m black --check --diff pyomo/contrib/gdpopt/branch_and_bound.py pyomo/contrib/gdpopt/tests/test_LBB.py pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py - passed, 3 files unchanged. Black emitted its Python 3.13 / Python 3.14 parser warning but completed successfully.
    • git diff --check upstream/main...HEAD -- pyomo/contrib/gdpopt/branch_and_bound.py pyomo/contrib/gdpopt/tests/test_LBB.py pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py - passed.
    • gh pr checks 3942 --repo Pyomo/pyomo - currently pending overall. Passing at review time: Read the Docs, lint/style-and-typos, linux/3.10/bare-env, linux/3.10/slim, linux/3.11/mpi, linux/3.12/numpy2. Remaining GitHub Actions matrix jobs and Jenkins inspection are still pending.
  5. Merge as-is: I do not see code changes needed from review, but I would wait for the pending required CI/Jenkins statuses before merging.

@bernalde
Copy link
Copy Markdown
Contributor Author

Coordination note for #3946:

This PR and #3946 fix different bugs. This one fixes the LBB time-limit result path for #3941; #3946 fixes continuous-node solver dispatch for #3945. #3946 does not supersede this PR.

I checked the combined patches with git merge-tree. The runtime changes are independent, but both PRs add imports/tests near the top of pyomo/contrib/gdpopt/tests/test_LBB.py, so the second PR merged will likely need a small conflict resolution there.

Recommendation: merge this PR first since it is smaller and already through the GitHub Actions matrix, then refresh/rebase #3946 afterward.

Copy link
Copy Markdown
Contributor

@emma58 emma58 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the long run we should think about moving the GDPopt solvers onto the new contrib.solver results object, but this looks good for now--it fixes a dumb bug in LBB leftover from a long ago GDPopt rewrite.


class TestMindtPyGOATimeLimit(unittest.TestCase):
def test_goa_time_limit_sets_solver_results_condition(self):
from pyomo.contrib.mindtpy.global_outer_approximation import MindtPy_GOA_Solver
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move the import to the top of the file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in ede4b3787 by moving MindtPy_GOA_Solver to the file-level imports in pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py. The GOA regression still passes with pytest -q pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py::TestMindtPyGOATimeLimit::test_goa_time_limit_sets_solver_results_condition.

@bernalde
Copy link
Copy Markdown
Contributor Author

Review comments addressed.

Commits pushed:

  • ede4b3787 - Address MindtPy GOA test import review

Main changes made:

  • Moved MindtPy_GOA_Solver import from inside test_goa_time_limit_sets_solver_results_condition() to the top-level import block in pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py.

Tests run:

  • pytest -q pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py::TestMindtPyGOATimeLimit::test_goa_time_limit_sets_solver_results_condition - passed, 1 passed in 0.79s.
  • pytest -q pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py - passed, 29 passed in 1.59s.
  • pytest -q pyomo/contrib/gdpopt/tests/test_LBB.py::TestGDPopt_LBB_TimeLimit::test_time_limit_returns_pyomo_results_object pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py::TestMindtPyGOATimeLimit::test_goa_time_limit_sets_solver_results_condition - passed, 2 passed in 0.57s.
  • python -m black --check --diff pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py - passed, 1 file unchanged. Black emitted its Python 3.13 / Python 3.14 parser warning but completed successfully.

Comments intentionally not addressed:

Remaining risks or follow-up items:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GDPopt LBB time-limit path calls stale _get_final_results_object

3 participants